Skip to content

Add static members to the extended completion#3949

Merged
KevinRansom merged 2 commits into
dotnet:masterfrom
vasily-kirichenko:static-members-in-extended-completion
Nov 20, 2017
Merged

Add static members to the extended completion#3949
KevinRansom merged 2 commits into
dotnet:masterfrom
vasily-kirichenko:static-members-in-extended-completion

Conversation

@vasily-kirichenko

Copy link
Copy Markdown
Contributor

This fixes #3948

image

Comment thread src/fsharp/vs/ServiceAssemblyContent.fs Outdated

let thisRequiresQualifierAccess =
if entity.IsFSharp && Symbol.hasAttribute<RequireQualifiedAccessAttribute> entity.Attributes then
if (entity.IsFSharp && Symbol.hasAttribute<RequireQualifiedAccessAttribute> entity.Attributes) || entity.IsClass then

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should count value types and simple types that may have static members as well.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know :) The problem that static members should always be prefixed with the type (any type) name, but module values - if the module is RQA.

@auduchinok

Copy link
Copy Markdown
Member

@vasily-kirichenko Do we need all members? I remember ReSharper adding such completion for properties/fields only and am OK with it this way. I don't think I'd call a method this way from completion.

@vasily-kirichenko

Copy link
Copy Markdown
Contributor Author

Fixed.

@auduchinok why not?

image

@auduchinok

Copy link
Copy Markdown
Member

@vasily-kirichenko I'm a bit afraid of how many members there may be when lots of assemblies are referenced. Probably a good compromise would be to make it optional.

@cartermp

Copy link
Copy Markdown
Contributor

We'll do some acceptance testing on projects with a bunch of assemblies before pulling this. I imagine perf would be proportional to how it is today (unless there are tons of solutions out there which use crazy amounts of static members compared with instance members, which I doubt is very common).

@vasily-kirichenko

Copy link
Copy Markdown
Contributor Author

It's ready.

@vasily-kirichenko vasily-kirichenko changed the title [WIP] Add static members to the extended completion Add static members to the extended completion Nov 15, 2017
@saul

saul commented Nov 15, 2017

Copy link
Copy Markdown
Contributor

Why are we showing static members? Surely these should only show after you’ve dotted into the type that has the static members?

@smoothdeveloper

Copy link
Copy Markdown
Contributor

@saul, it is the same idea of having the namespaces open when you put a type name from a namespace which is not in scope.

Since F# has few places where static member needs the explicit prefix (in instance members of a same class) and in more general cases (like looking for an extension method which is not in scope), I think that feature is useful.

@smoothdeveloper

Copy link
Copy Markdown
Contributor

@vasily-kirichenko thanks for that feature! is there a special handling for the extension methods already or can we consider adding it?

Here is what Resharper does in C# code, here it shows extension methods that are not in scope.

image

I tried the same in recently updated VS2017 and it doesn't have that, but maybe I need to install a nighlty?

@vasily-kirichenko

Copy link
Copy Markdown
Contributor Author

@smoothdeveloper extention members are not supported. To do it we need to collect all of them in a separate list in assembly content cache and mix into after-dot completion.

@vasily-kirichenko

vasily-kirichenko commented Nov 16, 2017

Copy link
Copy Markdown
Contributor Author

@saul because ReSharper does this and it would be great to be on par with C# completion in Rider. Personally I cannot say if this feature is useful or not, will see.

@vasily-kirichenko

Copy link
Copy Markdown
Contributor Author

Ooookey, it's ready.

@KevinRansom KevinRansom left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is neat. Thank you

@KevinRansom KevinRansom merged commit ae6f593 into dotnet:master Nov 20, 2017
auduchinok added a commit to auduchinok/fsharp that referenced this pull request Jan 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Extended completion should contain extension and static members

6 participants